Add `WithPgWeb` #5098

Alirexaa · 2024-07-28T20:16:19Z

In this PR, we add pgweb (https://github.com/sosedoff/pgweb) a web-based admin dashboard for PostgreSQL.

using WithPgWeb, We collect all registered PostgresDatabaseResource instances and create a configuration file per instance, then bind the location of these files to the pgweb container bookmark directory. for more information about pgweb bookmark you can see https://github.com/sosedoff/pgweb/wiki/Server-Connection-Bookmarks.

We used the Eventing API and subscribed AfterEndpointsAllocatedEvent to create configuration files.

We used pgweb connect API for functional testing to ensure pgweb is configured correctly.

var testConnectionApiUrl = $"{pgWebEndpoint.Scheme}://{pgWebEndpoint.Host}:{pgWebEndpoint.Port}/api/connect";

var pipeline = new ResiliencePipelineBuilder().AddRetry(new Polly.Retry.RetryStrategyOptions
{
    Delay = TimeSpan.FromSeconds(2),
    MaxRetryAttempts = 10,
}).Build();

await pipeline.ExecuteAsync(async (ct) =>
{
    var httpContent = new MultipartFormDataContent
    {
        { new StringContent(dbName), "bookmark_id" }
    };

    client.DefaultRequestHeaders.Add("x-session-id", Guid.NewGuid().ToString());

    var response = await client.PostAsync(testConnectionApiUrl, httpContent, ct)
        .ConfigureAwait(false);

    response.EnsureSuccessStatusCode();
}, cts.Token).ConfigureAwait(false);

close: #3081

Microsoft Reviewers: Open in CodeFlow

Aspire.sln

playground/PostgresEndToEnd/PostgresEndToEnd.AppHost/Program.cs

src/Aspire.Hosting.PostgreSQL/PgAdminConfigWriterHook.cs

src/Aspire.Hosting.PostgreSQL/PostgresBuilderExtensions.cs

src/Aspire.Hosting.PostgreSQL/PostgresContainerImageTags.cs

mitchdenny · 2024-07-29T04:06:04Z

@Alirexaa this is looking pretty good. A few minor things. We'll need a docs issue/PR filed for this as well (probably just updating the existing doc page for Postgres).

Alirexaa · 2024-07-29T05:35:00Z

@mitchdenny besides these things, this PR needs more work to do, right now bookmarks populate in pgweb but when selecting one of those to connect, connection refuse occur.
Also need to write some tests

…ithPgweb

Alirexaa · 2024-08-08T19:57:06Z

To pass all tests we need to import docker.io/sosedoff/pgweb/latest to netaspireci.azurecr.io

Alirexaa · 2024-08-09T18:10:15Z

@eerhardt could you please import docker.io/sosedoff/pgweb/latest to make ci green?

eerhardt · 2024-08-09T20:52:04Z

src/Aspire.Hosting.PostgreSQL/PostgresContainerImageTags.cs

@@ -11,4 +11,7 @@ internal static class PostgresContainerImageTags
    public const string PgAdminRegistry = "docker.io";
    public const string PgAdminImage = "dpage/pgadmin4";
    public const string PgAdminTag = "8.8";
+    public const string PgWebRegistry = "docker.io";
+    public const string PgWebImage = "sosedoff/pgweb";
+    public const string PgWebTag = "latest";


We've been not using latest tags as much as possible due to non-determinism. See examples:

[tests] Hosting.Container tests failing with /app/static/cachebuster.txt: No such file or directory #4828

Failing SqlServer, and SqlClient tests #5057

now we use the 0.15.0 tag,Could you please import docker.io/sosedoff/pgweb:0.15.0?

I can do that.

@eerhardt already did it!

src/Aspire.Hosting.PostgreSQL/PgWebConfigWriterHook.cs

eerhardt · 2024-08-09T20:56:06Z

src/Aspire.Hosting.PostgreSQL/PgWebConfigWriterHook.cs

+    {
+        var adminResource = appModel.Resources.OfType<PgWebContainerResource>().Single();
+        var serverFileMount = adminResource.Annotations.OfType<ContainerMountAnnotation>().Single(v => v.Target == "/.pgweb/bookmarks");
+        var postgresInstances = appModel.Resources.OfType<PostgresDatabaseResource>();


Do we want to do this for all PostgresDatabaseResource instances? Or just the ones that had .WithPgWeb() called on them?

do we need to introduce a new annotation to make a relation between PgWebContainerResource and PostgresDatabaseResource or maybe something like this

public sealed class PgWebContainerResource(string name,PostgresDatabaseResource postgresDatabaseResource) : ContainerResource(name) { public PostgresDatabaseResource PostgresDatabaseResource { get; } = postgresDatabaseResource; }

@eerhardt, what about this?
We have same behavior for pgadmin.

I guess if the behavior is the same as pgadmin, then I'm fine. If we feel we should have a different behavior for both - log an issue.

We have an open issue for that. see #2060.

src/Aspire.Hosting.PostgreSQL/PostgresBuilderExtensions.cs

eerhardt · 2024-08-09T20:59:50Z

src/Aspire.Hosting.PostgreSQL/PostgresBuilderExtensions.cs

+            builder.ApplicationBuilder.Services.TryAddLifecycleHook<PgWebConfigWriterHook>();
+
+            containerName ??= $"{builder.Resource.Name}-pgweb";
+            var dir = Directory.CreateTempSubdirectory().FullName;


How does this directory get cleaned up?

do we have a hook or event when a resource stopped/finished?

I think we should add one API for that in IDistributedApplicationLifecycleHook

This is actually a problem right now. @karolz-ms interested in your thoughts here. We sometimes drop temporary files in disk and I'd like to be able to try and ensure that they are cleaned up. We can probably have some kind of temporary file register that goes and does clean up but its highly likely that some of those files/directories are going to be locked because things like containers are spinning down.

This might be one of those things that DCP could help with?

Yes, I think DCP would be a good place for logic that deals with "cleanup" tasks like removing temporary log files.

There are probably many ways to do this, but the simplest one that will work today, is to put them inside the DCP session folder (which the app host runtime knows about). That folder is automatically deleted when DCP is shutting down. Just make sure the name is reasonably unique when creating the file.

(we might be missing an API for the extensions that will tell them what the "temporary folder for the current application run" is)

We could do that too. Feel free to open an issue and we can add it to our backlog for DCP.

what is our approach for cleanup temp files for this PR?

@mitchdenny - would this be a good usage of the new Eventing API? We could have an event that fires when the resource is shut down.

what is our approach for cleanup temp files for this PR?

Since we already have the same issue with pgadmin:

aspire/src/Aspire.Hosting.PostgreSQL/PostgresBuilderExtensions.cs

Lines 101 to 111 in 4b4ebce

.WithBindMount(Path.GetTempFileName(), "/pgadmin4/servers.json")

.ExcludeFromManifest();

builder.ApplicationBuilder.Eventing.Subscribe<AfterEndpointsAllocatedEvent>((e, ct) =>

{

var serverFileMount = pgAdminContainer.Annotations.OfType<ContainerMountAnnotation>().Single(v => v.Target == "/pgadmin4/servers.json");

var postgresInstances = builder.ApplicationBuilder.Resources.OfType<PostgresServerResource>();

var serverFileBuilder = new StringBuilder();

using var stream = new FileStream(serverFileMount.Source!, FileMode.Create);

I'd be fine with continuing here to just create temp files and leave them, for now, until we get the clean up capability.

Yes ... although it wouldn't be 100%. If someone kills the process via the debugger (most common scenario) I don't think we guarantee that this is run. But better than nothing I guess.

tests/Aspire.Hosting.PostgreSQL.Tests/AddPostgresTests.cs

src/Aspire.Hosting.PostgreSQL/PgWebConfigWriterHook.cs

playground/PostgresEndToEnd/PostgresEndToEnd.AppHost/Program.cs

eerhardt · 2024-08-15T19:39:24Z

src/Aspire.Hosting.PostgreSQL/PgWebConfigWriterHook.cs

+    {
+        var adminResource = appModel.Resources.OfType<PgWebContainerResource>().Single();
+        var serverFileMount = adminResource.Annotations.OfType<ContainerMountAnnotation>().Single(v => v.Target == "/.pgweb/bookmarks");
+        var postgresInstances = appModel.Resources.OfType<PostgresDatabaseResource>();


I guess if the behavior is the same as pgadmin, then I'm fine. If we feel we should have a different behavior for both - log an issue.

radical · 2024-08-15T20:45:55Z

Can you please add some information in PR description, maybe even just links to pgweb?

eerhardt · 2024-08-16T15:58:19Z

@eerhardt could you please import docker.io/sosedoff/pgweb/latest to make ci green?

I've pushed it to the netaspireci.azurecr.io registry. However, I don't believe the test will pass because the CI machines don't have Docker Desktop installed, which means there isn't network communication between the containers.

Alirexaa · 2024-08-16T16:38:50Z

@eerhardt could you please import docker.io/sosedoff/pgweb/latest to make ci green?

I've pushed it to the netaspireci.azurecr.io registry. However, I don't believe the test will pass because the CI machines don't have Docker Desktop installed, which means there isn't network communication between the containers.

What can I do for that?

eerhardt · 2024-08-16T18:01:05Z

What can I do for that?

For now we've been disabling the test in CI until the problem can be resolved.

radical · 2024-08-16T18:01:43Z

/azp run

azure-pipelines · 2024-08-16T18:01:55Z

Azure Pipelines successfully started running 1 pipeline(s).

eerhardt

LGTM. Thanks for this contribution!

Add WithPgWeb

71a52a1

dotnet-issue-labeler bot added the area-integrations Issues pertaining to Aspire Integrations packages label Jul 28, 2024

dotnet-policy-service bot added the community-contribution Indicates that the PR has been added by a community member label Jul 28, 2024

mitchdenny marked this pull request as ready for review July 29, 2024 03:18